Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Run] Improve error MessageBox #34564

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

davidegiacometti
Copy link
Collaborator

@davidegiacometti davidegiacometti commented Sep 3, 2024

Summary of the Pull Request

This is not a fix to the error, but an improvement to MessageBox error title since it's not clear to the user that the it's related to PT Run.
This was partially addressed by #25095, but not for all the errors.

image

image

image

PR Checklist

Detailed Description of the Pull Request / Additional comments

  • Assessed the MessageBox.Show(...) errors that could be triggered on startup, detecting which need to explicit call out "PT Run"
  • Aligned loading/initialization error messages.
  • Don't think that IPublicAPI.ShowMsg(...) dialogs need "PT Run" in title since they are usually showed as a response to a user interaction (example click on a result or action).

Validation Steps Performed

@htcfreek
Copy link
Collaborator

htcfreek commented Sep 4, 2024

@davidegiacometti
There are two plugin init error msg boxes. Did you check if the title is used in both of them?

@davidegiacometti
Copy link
Collaborator Author

I'll check. Thanks for pointing out this.

@htcfreek

This comment has been minimized.

@htcfreek
Copy link
Collaborator

htcfreek commented Sep 4, 2024

Screenshot_20240904-190429_GitHub.jpg

@davidegiacometti
Copy link
Collaborator Author

I have verified and one plugin error dialog was addressed in #25095.
This PR aim to fix the remaining and another related to the hotkey.

@davidegiacometti davidegiacometti force-pushed the users/davidegiacometti/run-error-msgbox branch from 9ee52f5 to 67be391 Compare September 5, 2024 20:30
@htcfreek
Copy link
Collaborator

htcfreek commented Sep 6, 2024

@davidegiacometti
Thoughts regarding the init error message:

  1. We should update the content of the init error message to be aligned with the title. (Loading vs initializing.)
  2. Can we improve the text to be in line with the loading error text? (First list the affected plugins and in the next paragraph give some advice what to do.) That makes it easier to understand what plugins are affected, I think.
  3. I think we should use the same advice like in the loading error message.

Copy link
Collaborator

@htcfreek htcfreek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM . ❤️ This is a big quality of life improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants